fix: skip unnecessary test runs for bot-generated and non-critical PRs #1339
Conversation
Fix/skip unnecessary tests
docs: update README.md for testing
Update pr-testing-with-test-project.yml
Update pr-testing-with-test-project.yml
AdityaP700
requested review from
derberg,
magicmatatjahu,
jonaslagoni and
asyncapi-bot-eve
as code owners
|
|
Hey @derberg ,done with the changes in the required file , do let me know of any further changes . Thanks |
AdityaP700
changed the title
AdityaP700
changed the title
AdityaP700
changed the title
|
LGTM :) |
|
Hey @derberg can you run the workflow. |
There was a problem hiding this comment.
Amazing PR @AdityaP700 just few suggestions to improve it more.
| should-test: | ||
| runs-on: ubuntu-latest | ||
| outputs: | ||
| run_tests: ${{ steps.check.outputs.run_tests }} | ||
| steps: | ||
| - name: Check if tests should run | ||
| id: check | ||
| run: | | ||
| # Skip for bot PRs with specific titles | ||
| if [[ "${{ github.actor }}" == "asyncapi-bot" && "${{ github.event.pull_request.title }}" =~ ^(ci:\ update|chore\(release\):) ]] || \ | ||
| [[ "${{ github.actor }}" == "allcontributors[bot]" && "${{ github.event.pull_request.title }}" =~ ^docs: ]]; then | ||
| echo "run_tests=false" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "run_tests=true" >> $GITHUB_OUTPUT | ||
| fi | ||
|
|
There was a problem hiding this comment.
Adding a new job on which the actual job to be done is dependent won't increase job setup time?
Instead, how's about adding an if clause just like it is done here
There was a problem hiding this comment.
I did tried adding but thought of optimising it a bit thus made it likewise ,anyway will look into it .
| test: | ||
| if: github.event.pull_request.draft == false | ||
| needs: should-test | ||
| if: needs.should-test.outputs.run_tests == 'true' && github.event.pull_request.draft == false |
There was a problem hiding this comment.
| if: needs.should-test.outputs.run_tests == 'true' && github.event.pull_request.draft == false | |
| if: needs.should-test.outputs.run_tests == true && github.event.pull_request.draft == false |
wrapping true in single quotes makes it string, although eventually, it results in a true statement (as strings are also considered to be true) so the workflow works as expected. But it's a good practice to use Booleans for truthy and falsy statements.
There was a problem hiding this comment.
Alright got it !! i took the assumptions so did such changes but anyway will resolve it .
README.md
Outdated
| @@ -8,7 +8,7 @@ This is a Monorepo managed using [Turborepo](https://turbo.build/) and contains | |||
|
|
|||
| 1. [Hooks](apps/hooks): This library contains generator filters that can be reused across multiple templates, helping to avoid redundant work. Hooks are designed to let template developers hook into the template generation process. For example, one can create a hook code that will be automatically invoked right after the template generation process has ended. | |||
|
|
|||
| 1. [Nunjucks-filters](apps/nunjucks-filters): This library contains generator filters that can be reused across multiple templates, helping to avoid redundant work. These filters are designed specifically for Nunjucks templates and are included by default with the generator, so there's no need to add them to dependencies separately. | |||
| 1. [Nunjucks-filters](apps/nunjucks-filters): This library contains generator filters that can be reused across multiple templates helping to avoid redundant work. The filters are designed specifically for Nunjucks templates and are included by default with the generator, so there's no need to add them to dependencies separately. | |||
There was a problem hiding this comment.
Pardon me : will discard the commit of it !!
|
|
||
| jobs: | ||
| test: | ||
| if: github.event.pull_request.draft == false |
There was a problem hiding this comment.
this should stay, we do not want to run this on drafts
There was a problem hiding this comment.
Alright then ,will fix it accordingly.
| run: echo "shouldrun=true" >> $GITHUB_OUTPUT | ||
| shell: bash | ||
|
|
||
| - if: steps.should_run.outputs.shouldrun == true |
There was a problem hiding this comment.
let's use 'true' like in other workflows.
technically output of run: echo "shouldrun=true" >> $GITHUB_OUTPUT is string -> https://docs.github.com/en/actions/sharing-automations/creating-actions/metadata-syntax-for-github-actions#outputs-for-docker-container-and-javascript-actions
so also technically please compare with true that is represented as string
There was a problem hiding this comment.
I was confused at first as everywhere i was getting the response to use it as a string but since i was adviced to use it as a boolean expression so thought to implement it . Well will update it soon
| @@ -4,10 +4,17 @@ name: Test using test project | |||
| on: | |||
| pull_request: | |||
| types: [opened, reopened, synchronize, ready_for_review] | |||
| paths: | |||
There was a problem hiding this comment.
this is an interesting approach, any examples in production? how it works for others? like what was the inspiration?
so far we used only the "inclusion" part, not "exclusion"
There was a problem hiding this comment.
This approach was inspired by use cases where filtering out certain paths or scenarios is critical to avoid unintended actions. The idea aligns with scenarios where 'exclusion' can complement 'inclusion' for more precise control in the reference of https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#example-including-paths : so thus had the idea of implementing the same here in the workflow.
|
Description
Related Issue(s)
Fixes Improve testing speed by skipping it for certain PRs #1335
Testing